Don't emit Unevaluated from const_eval
#56723
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
r? @RalfJung |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
| "_Self".to_owned(), | ||
| Some(format!("[{}; _]", self.tcx.type_of(def.did).to_string())), | ||
| )); | ||
| } |
There was a problem hiding this comment.
So we don't print anything when it is not evaluated? That is different from the previous code.
I have no idea where this code is called, just pointing out a functional change. ;)
There was a problem hiding this comment.
oops indeed. This is just for producing more diagnostics, but still. I found a nice way to fix it without more duplication.
|
Were |
Yes they were interned before. Yes we now intern twice. I'll check if we can get away with not interning |
| @@ -376,6 +376,11 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { | |||
| constant, location | |||
| ); | |||
|
|
|||
| let literal = match constant.literal { | |||
| ty::LazyConst::Evaluated(lit) => lit, | |||
| ty::LazyConst::Unevaluated(..) => return, | |||
There was a problem hiding this comment.
Is it really correct to return early here in case of Unevaluated?
There was a problem hiding this comment.
This is #46702
I don't believe this PR makes the situation any more problematic than it was.
There was a problem hiding this comment.
Okay.
Cc @eddyb would be good if you could take a quick look at some point, I have no idea what this is doing.^^
If we don't, that makes me wonder about whether this refactoring is worth it. :/ |
Sorry, I misspoke: We might not have any |
This comment has been minimized.
This comment has been minimized.
|
This should theoretically not have a negative perf impact, but I've guessed wrong with this kind of change before. let's see what perf says @bors try |
|
⌛ Trying commit b01765fdfdbec1b70d7d41dccf7db02467721eeb with merge 6ab2e74a91c27b914174a1c9db557e41235bce60... |
|
@rust-timer build 6ab2e74a91c27b914174a1c9db557e41235bce60 |
|
Success: Queued 6ab2e74a91c27b914174a1c9db557e41235bce60 with parent 7d03617, comparison URL. |
|
Finished benchmarking try commit 6ab2e74a91c27b914174a1c9db557e41235bce60 |
|
💥 Test timed out |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
r=me modulo rebase |
nikomatsakis
added
S-waiting-on-author
|
@bors r=nikomatsakis |
|
📌 Commit 03b8928 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - status-appveyor, status-travis |
| impl<'a, 'tcx> Lift<'tcx> for &'a List<$ty> { | ||
| type Lifted = &'tcx List<$lifted>; | ||
| fn lift_to_tcx<'b, 'gcx>(&self, tcx: TyCtxt<'b, 'gcx, 'tcx>) -> Option<Self::Lifted> { | ||
| if self.is_empty() { |
| self.require_sized(constant.ty, traits::ConstSized); | ||
| if let ConstValue::Unevaluated(def_id, substs) = constant.val { | ||
| fn compute_array_len(&mut self, constant: ty::LazyConst<'tcx>) { | ||
| if let ty::LazyConst::Unevaluated(def_id, substs) = constant { |
There was a problem hiding this comment.
Why was this change made? If I called something const it's because it's meant to be used with const generics in the future.
This needs to be renamed back, and used for all ty::Consts in all Substs.
The lack of WF const handling probably allows a lot of unintended const expressions.
cc @varkor @yodaldevoid
cc @eddyb @RalfJung